Skip to content

Default to capturing mic + all system audio. - #39

Open
aaugustin wants to merge 1 commit into
paberr:mainfrom
aaugustin:feature/improve-default-config
Open

Default to capturing mic + all system audio.#39
aaugustin wants to merge 1 commit into
paberr:mainfrom
aaugustin:feature/improve-default-config

Conversation

@aaugustin

Copy link
Copy Markdown
Contributor

Most users record meetings they're participating in, so mic=true and
capture_mode="all" out of the box avoids missing your own audio and
skips the source picker prompt.

--no-mic allows opting-out. With --no-mic, --mic-device is rejected,
as there's no sensible behavior for this combination.

Fix #36.

Most users record meetings they're participating in, so mic=true and
capture_mode="all" out of the box avoids missing your own audio and
skips the source picker prompt.

--no-mic allows opting-out. With --no-mic, --mic-device is rejected,
as there's no sensible behavior for this combination.

Fix paberr#36.
Copilot AI review requested due to automatic review settings July 15, 2026 20:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates ownscribe’s out-of-the-box audio capture behavior to better match the primary “record a meeting you’re in” use case by enabling microphone capture and recording all system audio without prompting by default, while adding an explicit CLI opt-out.

Changes:

  • Change default audio config to mic = true and capture_mode = "all" (avoids missing the user’s own voice and avoids the source picker prompt).
  • Update CLI to support --mic/--no-mic and to reject the --no-mic + --mic-device combination.
  • Refresh tests and README examples to reflect the new defaults and flags.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/test_pipeline.py Updates recorder-construction expectations to match new default mic + capture mode.
tests/test_config.py Adjusts default-config assertions for mic and capture_mode.
tests/test_cli.py Updates CLI parsing tests for --no-mic and adds coverage for invalid flag combinations.
src/ownscribe/config.py Changes default audio settings in both TOML template and dataclass defaults.
src/ownscribe/cli.py Implements --mic/--no-mic tri-state override behavior and validates incompatible combinations.
README.md Updates usage examples and config snippet to match the new default capture behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ownscribe/cli.py
Comment on lines +101 to 107
if mic is False and mic_device:
raise click.UsageError("--no-mic and --mic-device cannot be used together.")
if mic is not None:
config.audio.mic = mic
if mic_device:
config.audio.mic = True
config.audio.mic_device = mic_device
Comment thread README.md
1. Capture system audio and your microphone until you press Ctrl+C (or auto-stop after 5 minutes of silence)
2. Transcribe with WhisperX
3. Summarize with your local LLM
4. Save everything to `~/ownscribe/YYYY-MM-DD_HHMMSS/`
Comment thread tests/test_cli.py
Comment on lines +33 to +39
def test_no_mic_flag(self):
runner = CliRunner()
with _mock_config(), mock.patch("ownscribe.pipeline.run_pipeline") as mock_run:
result = runner.invoke(cli, ["--mic"])
result = runner.invoke(cli, ["--no-mic"])
assert result.exit_code == 0
config = mock_run.call_args[0][0]
assert config.audio.mic is True
assert config.audio.mic is False

@paberr paberr left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the work!
Besides the Copilot & my comments, I think the README needs some updating:

  1. line 45 still mentions the old default and --mic
  2. I'm not sure if the permissions described in 68 are still correct or if there's a microphone prompt too?
  3. The mute hint "press 'm'" is on the --no-mic example where it doesn't actually apply.

Comment thread src/ownscribe/cli.py
Comment on lines +101 to +104
if mic is False and mic_device:
raise click.UsageError("--no-mic and --mic-device cannot be used together.")
if mic is not None:
config.audio.mic = mic

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: I think --no-mic doesn't actually disable the mic when mic_device is set in config.toml:

  1. --no-mic sets config.audio.mic = False, but config.audio.mic_device keeps its value from the config file.
  2. CoreAudioRecorder.start() re-enables the mic from the device name alone (if self._mic or self._mic_device: cmd.append("--mic")).
  3. So the helper is still launched with --mic --mic-device "USB Mic" and we record the mic anyway.

Possible fix: clear config.audio.mic_device when --no-mic is given, e.g.

if mic is not None:
    config.audio.mic = mic
    if mic is False:
        config.audio.mic_device = ""

(same as CoPilot's comment below I just noticed)

Comment thread tests/test_cli.py
Comment on lines +33 to +39
def test_no_mic_flag(self):
runner = CliRunner()
with _mock_config(), mock.patch("ownscribe.pipeline.run_pipeline") as mock_run:
result = runner.invoke(cli, ["--mic"])
result = runner.invoke(cli, ["--no-mic"])
assert result.exit_code == 0
config = mock_run.call_args[0][0]
assert config.audio.mic is True
assert config.audio.mic is False

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small things on the tests here, non-blocking:

  1. With test_mic_flag renamed, we no longer test that an explicit --mic overrides mic = false from the config file. Maybe keep a --mic test next to the --no-mic one?
  2. There's also no test that --mic-device alone implies mic = True — that implication now sits on its own line in cli.py and could regress silently.

Once the mic_device interaction above is fixed, a test for config-set mic_device + --no-mic would be a nice-to-have too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve default config for audio capture?

3 participants